Skip to content

Conversation

marclucraft
Copy link
Contributor

@marclucraft marclucraft commented Jul 24, 2025

#348 visually unchecked the box on post, but a page refresh results in the checkbox being checked again.


This change is Reviewable

@marclucraft marclucraft requested a review from sherwinski July 24, 2025 13:24
@sherwinski
Copy link
Contributor

My understanding was that the current behavior was what was always intended, i.e. so long as the "Send notification" setting is enabled from the admin page then the box should be enabled, even after refresh. Happy to be wrong on that but figured I'd at least double check.

@rfischmann
Copy link

My understanding was that the current behavior was what was always intended, i.e. so long as the "Send notification" setting is enabled from the admin page then the box should be enabled, even after refresh. Happy to be wrong on that but figured I'd at least double check.

If I may share my opinion, at least based on our work at MacMagazine, it would be a disaster if the checkbox was enabled by default even on posts that are already published. We're a multi-person team and we constantly edit posts from each other to add new information, fix mistakes, etc. It would be a nightmare for us to always have to remember to uncheck that option. We only fire new pushes for previous posts when the edit is very relevant and we want to let our audience know that the post was edited.

That's one of the reasons we're still on plugin version 2.4.4. It works exactly that way.

@sherwinski
Copy link
Contributor

@rfischmann That makes sense, thanks for the feedback!

@rfischmann
Copy link

Hey guys, is there any ETA for a plugin update with this fix? Thanks!

@sherwinski
Copy link
Contributor

Hey @rfischmann, I'm just wrapping up a couple of other tasks before moving on to this. We will you keep you updated here with any changes.

Copy link
Contributor

@sherwinski sherwinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- will move to merge and release

@sherwinski sherwinski merged commit 42f1ed4 into main Aug 8, 2025
2 checks passed
@sherwinski sherwinski deleted the send-notification-meta-checkbox branch August 8, 2025 20:33
@sherwinski sherwinski mentioned this pull request Aug 8, 2025
@sherwinski
Copy link
Contributor

@rfischmann this is now released in https://github.com/OneSignal/OneSignal-WordPress-Plugin/releases/tag/v3.2.1 🚀
Please feel free to reach out if you have any other questions or concerns.

@rfischmann
Copy link

It seems fine now, @sherwinski, thanks! Just a small tweak for a future update: the checkbox still says "Send notification when post is published" when the post is already published. In that case, it should change to "Send notification when post is edited".

Else, just show "Send notification when post is published/edited" in either case.

@sherwinski
Copy link
Contributor

Good call out @rfischmann -- I'll make an adjust to that in the next version. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants